Skip to content

♻️ Refactor: migrate more aiohttp app keys to type-safe web.AppKey (follow up) #8452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Oct 6, 2025
Merged

♻️ Refactor: migrate more aiohttp app keys to type-safe web.AppKey (follow up) #8452

merged 15 commits into from
Oct 6, 2025
+77 −61

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 3, 2025 •
edited
Loading

What do these changes do?

Follows up refactoring webserver's app state keys from strings to web.AppKey (see #7734)

Application state key refactoring:

  • Replaced string-based keys with web.AppKey objects in the long-running tasks module, introducing APP_LONG_RUNNING_MANAGER_APPKEY and RQT_LONG_RUNNING_TASKS_CONTEXT_APPKEY for safer access to manager and task context objects (_manager.py, _request.py, _server.py, __init__.py, tests, and plugins). [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]
  • Removed obsolete constants and updated usages in the resource manager module, introducing CLIENT_SOCKET_REGISTRY_APPKEY for registry access (_constants.py, plugin.py, registry.py). [1] [2] [3] [4] [5]
  • Refactored the project permalink service to use PROJECT_PERMALINK_FACTORY_APPKEY instead of a string key for the permalink factory coroutine (_permalink_service.py). [1] [2] [3]
  • Updated socketio utility module to use APP_CLIENT_SOCKET_DECORATED_HANDLERS_APPKEY and APP_CLIENT_SOCKET_SERVER_APPKEY for handler and server references, replacing string keys (_utils.py). [1] [2]

Related issue/s

How to test

Dev-ops

None

@pcrespov pcrespov added this to the Cheops milestone Oct 3, 2025
@pcrespov pcrespov self-assigned this Oct 3, 2025
@pcrespov pcrespov added a:webserver webserver's codebase. Assigning the area is particularly useful for bugs a:services-library issues on packages/service-libs t:maintenance Some planned maintenance work labels Oct 3, 2025
@pcrespov pcrespov changed the title ♻️ Is7734/more app keys ♻️ Refactor: migrate more aiohttp app keys to type-safe web.AppKey (follow up) Oct 3, 2025
@codecov
Copy link

codecov bot commented Oct 3, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.21%. Comparing base (6b176ba) to head (55a8ca3).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8452      +/-   ##
==========================================
+ Coverage   87.18%   87.21%   +0.03%     
==========================================
  Files        1984     1483     -501     
  Lines       77284    62291   -14993     
  Branches     1333      678     -655     
==========================================
- Hits        67377    54330   -13047     
+ Misses       9508     7723    -1785     
+ Partials      399      238     -161
Flag Coverage Δ
integrationtests 64.16% <80.00%> (+3.62%) ⬆️
unittests 85.58% <100.00%> (-0.83%) ⬇️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library 70.92% <100.00%> (-0.05%) ⬇️
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.01% <ø> (+0.05%) ⬆️
agent 93.53% <ø> (ø)
api_server 91.78% <ø> (ø)
autoscaling 95.71% <ø> (ø)
catalog 92.36% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 91.78% <ø> (-0.57%) ⬇️
datcore_adapter 97.94% <ø> (ø)
director 75.81% <ø> (ø)
director_v2 90.95% <ø> (+5.62%) ⬆️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 90.43% <ø> (ø)
efs_guardian 89.74% <ø> (ø)
invitations 91.41% <ø> (ø)
payments 92.71% <ø> (ø)
resource_usage_tracker 92.15% <ø> (ø)
storage 86.74% <ø> (+0.29%) ⬆️
webclient ∅ <ø> (∅)
webserver 87.65% <100.00%> (+<0.01%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b176ba...55a8ca3. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pcrespov pcrespov marked this pull request as ready for review October 3, 2025 15:13
@pcrespov pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Oct 3, 2025
@pcrespov pcrespov enabled auto-merge (squash) October 3, 2025 15:13
@mergify
Copy link
Contributor

mergify bot commented Oct 3, 2025 •
edited
Loading

🧪 CI Insights

Here's what we observed from your CI run for 55a8ca3.

❌ Job Failures

Pipeline Job Health on master Retries 🔍 CI Insights 📄 Logs
CI system-tests Healthy 0 View View
unit-tests Healthy 0 View View

@pcrespov pcrespov force-pushed the is7734/more-app-keys branch from 2ba05b8 to 332b271 Compare October 3, 2025 17:30
pcrespov added 12 commits October 3, 2025 21:48
@pcrespov
🎨 Refactor socketio handler keys to use web.AppKey for improved clari…
c1d2388 
…ty and type safety
@pcrespov
🎨 Refactor long-running tasks manager key to use web.AppKey for consi…
f891dd8 
…stency
@pcrespov
🎨 Refactor long-running tasks context key to use RQT_LONG_RUNNING_TAS…
11e5856 
…KS_CONTEXT_APPKEY for consistency
@pcrespov
🎨 Refactor permalink factory key to use PROJECT_PERMALINK_FACTORY for…
fde8d59 
… consistency
@pcrespov
🎨 Refactor resource manager keys to use CLIENT_SOCKET_REGISTRY_APPKEY…
e932e34 
… for consistency
@pcrespov
🎨 Refactor long-running tasks context key to use LONG_RUNNING_TASKS_C…
cda3add 
…ONTEXT_REQKEY for consistency
@pcrespov
🎨 Refactor APP_CLIENT_SOCKET_SERVER_APPKEY definition for clarity and…
7ad98c2 
… consistency
@pcrespov
🎨 Refactor socket server key definitions for consistency
c0d24c7
@pcrespov
minor
b8d94a8
@pcrespov
🎨 Refactor long-running tasks context key for consistency and clarity
06e176b
@pcrespov
minor
9b149f0
@pcrespov
fix
ef92407
@pcrespov pcrespov force-pushed the is7734/more-app-keys branch from 332b271 to ef92407 Compare October 3, 2025 20:42
GitHK
GitHK approved these changes Oct 6, 2025
View reviewed changes
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

bisgaard-itis
bisgaard-itis approved these changes Oct 6, 2025
View reviewed changes
Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@pcrespov pcrespov disabled auto-merge October 6, 2025 15:54
@pcrespov pcrespov merged commit 5199e38 into ITISFoundation:master Oct 6, 2025
3 of 4 checks passed
@pcrespov pcrespov deleted the is7734/more-app-keys branch October 6, 2025 15:54
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 6, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

This was referenced Oct 28, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@giancarloromeo giancarloromeo giancarloromeo approved these changes

@GitHK GitHK GitHK approved these changes

@matusdrobuliak66 matusdrobuliak66 matusdrobuliak66 approved these changes

@bisgaard-itis bisgaard-itis bisgaard-itis approved these changes

@sanderegg sanderegg Awaiting requested review from sanderegg sanderegg is a code owner

Assignees

@pcrespov pcrespov

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:services-library issues on packages/service-libs a:webserver webserver's codebase. Assigning the area is particularly useful for bugs t:maintenance Some planned maintenance work

Projects

None yet

Milestone

Cheops

Development

Successfully merging this pull request may close these issues.

Address aiohttp NotAppKeyWarning: It is recommended to use web.AppKey instances for keys

5 participants

@pcrespov @giancarloromeo @GitHK @matusdrobuliak66 @bisgaard-itis